-
Notifications
You must be signed in to change notification settings - Fork 117
docs(l2): add integration tests section #5126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds documentation for running integration tests for the ethrex L2 project. The documentation includes prerequisites, environment setup instructions, and steps to run the integration tests.
- Added new integration tests documentation file
- Updated table of contents in the L2 developer introduction page
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/developers/l2/introduction.md | Added link to the new integration tests documentation |
| docs/developers/l2/integration-tests.md | New documentation explaining how to set up and run integration tests for ethrex L2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| > [!NOTE] | ||
| > ethrex's MPT trie implementation is path-based, and the database commit threshold is set to `128`. In simple words, the latter implies that the database only stores the state 128 blocks before the current one (e.g., if the current block is block 256, then the database stores the state at block 128), while the state of the blocks within lives in in-memory diff layers (which are lost during node shutdowns). | ||
| > In ethrex L2, this has a direct impact since if our sequencer seals batches with more than 128 blocks, it won't be able to retrieve the state previous to the first block of the batch being sealed because it was pruned; therefore, it won't be able to commit. | ||
| > To solve this, after a batch is sealed, we create a checkpoint of the database at that point to ensure the state needed at the time of commitment is available for the sequencer. | ||
| > For this test to be valuable, we need to ensure this edge case is covered. To ensure this is covered, we set up an L2 with batches of ~150 blocks by setting the flag `--block-producer.block-time` (specifies the interval in milliseconds in which we want our builder to build an L2 block) to 1 second, meaning that the L2 block builder will build blocks every 1 second; and by setting the flag `--committer.commit-time` (specifies the interval in milliseconds in which we want to commit to the L1) to 150s (2m 30s) to ensure that enough blocks are included in the batch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note is technical and valuable, but it's dense. It provides the necessary context to understand why we are configuring the L2 the way it is specified.
Any feedback on how to improve this part is more than welcome.
Co-authored-by: Copilot <[email protected]>
|
|
||
| ## Prerequisites | ||
|
|
||
| - This guide assumes that you have ethrex L2 installed. If you haven't done so, follow one of the installation methods in the [installation guide](https://docs.ethrex.xyz/getting-started/installation/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is going to be mostly for testing our releases, on the prerequisites besides sending people to the docs I suggest some extract like... if you need to run the latest realease just install it like this
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > ethrex's MPT trie implementation is path-based, and the database commit threshold is set to `128`. In simple words, the latter implies that the database only stores the state 128 blocks before the current one (e.g., if the current block is block 256, then the database stores the state at block 128), while the state of the blocks within lives in in-memory diff layers (which are lost during node shutdowns). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"MPT trie" is redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| During the execution of `ethrex l2 --dev`, a `.env` file is created and filled with environment variables containing contract addresses. This `.env` file is always needed for dev environments, so we need it for running the integration tests. Therefore, before running the integration tests, copy the `.env` file into `ethrex/cmd`: | ||
|
|
||
| ``` | ||
| cp .env ethrex/cmd | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this assume we cloned the ethrex repo and are not inside it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guide is missing a section explaining what "a passing test run" looks like.
|
|
||
| In this section, we will explain how to run integration tests for ethrex L2 with the objective of validating the correct functioning of our stack in our releases. For this, we will use ethrex as a local L2 dev node. | ||
|
|
||
| ## Prerequisites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should mention docker is required. I just ran everything without starting docker and it wasn't clear what was failing 🤡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wrong about this. Docker is not needed. The L1 dev node is started using the binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running the whole workflow gives me an error about the final balance of the base fee vault being 0. Requesting the current base fee vault address to the L2 node returns null, so I suspect the base fee vault is somehow being set to None.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the command options and also added some explanation on the newly added flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note on this:
We can run the integration tests without the following flags:
--block-producer.operator-fee-vault-address
--block-producer.l1-fee-vault-address
--block-producer.operator-fee-per-gas
Our integration tests verify that the fees are correct based on the network configuration.
If operator_fee is not set, the tests will assert that the final balance of the operator fee vault is 0 (the same applies to the L1 fee vault).
The base fee vault is a special case. If no vault is set, we need to manually pass INTEGRATION_TEST_SKIP_BASE_FEE_VAULT_CHECK=true to the integration tests (this could be updated to behave like the other fee checks).
We can then encourage testers to try different network configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR enables running the integration tests with any fee configuration without any additional steps.
Co-authored-by: Tomás Grüner <[email protected]>
Motivation
Testing the correct functioning of ethrex L2 in new releases or pre-releases is not as simple as it is for testing ethrex L1. It is not enough to run
ethrex l2 --devand see some non-error logs appear.We need a guide for ethrex L2 testers to follow, to ensure the correct functioning of this feature.
Description
Adds an integration tests section to the docs with a guide on how to properly test an ethrex L2 release.
Note
The troubleshooting section is TODO because it is the idea of this PR to collect feedback and fill it in case there's something to troubleshoot.